refactor: generate 1wg-charters files via celery#7428
Merged
rjsparks merged 12 commits intoietf-tools:mainfrom May 16, 2024
Merged
refactor: generate 1wg-charters files via celery#7428rjsparks merged 12 commits intoietf-tools:mainfrom
rjsparks merged 12 commits intoietf-tools:mainfrom
Conversation
Member
Author
|
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7428 +/- ##
==========================================
- Coverage 88.98% 88.92% -0.07%
==========================================
Files 291 294 +3
Lines 40717 41112 +395
==========================================
+ Hits 36233 36557 +324
- Misses 4484 4555 +71 ☔ View full report in Codecov by Sentry. |
Member
|
Acknowledging that there will be ordering and whitespace changes. That might disrupt some people doing diffs for a day, but the tradeoff seems the right one. |
…arters # Conflicts: # bin/hourly # ietf/utils/management/commands/periodic_tasks.py
rjsparks
approved these changes
May 16, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The main goal of this PR is to update the
1wg-charters.txtand1wg-charters-by-acronym.txtfiles via celery tasks instead of cron jobs. The cron job worked by using wget to retrieve a datatracker URL that generated the files. This was done once per hour and cached for an hour each time using theslowpagescache.As a result, the
slowpagescache is effectively always holding on to the data generated by the last run ofbin/hourlyand the view itself is always served from that cache except when the cron job regenerates it.This seems needlessly baroque, so I've refactored the work at generating the charters files entirely out of the views and into a task. The views then simply grab the file the task updates and hand that back in their response.
The new method has a bunch of benefits. It avoids collecting the group data twice (the views gathered the same data twice, the task does it only once). It avoids the small chance that the charter data is occasionally an extra hour out of date depending on system load (the cron job might make its request at very slightly less than an hour since the previously cached value). It entirely moves the purportedly expensive data gathering out of the view.
Important: I did a careful comparison of the output of the old and new code and there are a couple of differences. Most notably, the groups in
1wg-charters.txtare sorted alphabetically by acronym within each area. Before, they were (to my eye) random within the area. This seems like an improvement to me, but it is a change.Second, there were some
\r\nline endings in the old output - I assume they were bleeding in from the charters. These are converted to\nby the new rendering path. This seems like an absolute win to me, but again it's a change.Also important: We reviewed the
\r(aka CR) changes and found it was caused by CRLFs in some old draft titles, not in the charters. Added aclean_whitespacefilter to the title in the template to fix this. That replaces any run of ASCII whitespace / control characters (i.e., 0x00-0x1F) with a single space, then strips leading and trailing space. This ends up improving spacing / inappropriate line breaks in a couple dozen draft titles.